Fix URL validation and handling in URLUtil by sfahaddev · Pull Request #12800 · JabRef/jabref

sfahaddev · 2025-03-22T22:09:58Z

Closes #12186

Describe the changes you have made here: what, where, why, ...

What: Fixed URL validation and handling in the URLUtil class.
Where: Updated the isURL and create methods in URLUtil and added/modified tests in URLUtilTest.
Why: To resolve the issue where relative URLs (e.g., www.example.com) were not being handled correctly, and to ensure proper validation of URLs with various protocols (e.g., http://, https://, ftp://, file://).

Changes:

isURL Method:
- Removed unnecessary exception handling (MalformedURLException and IllegalArgumentException).
- Now only catches URISyntaxException to correctly identify invalid URLs.
- Ensures that only absolute URLs are considered valid.
create Method:
- Added logic to prepend http:// only to URLs without a protocol (e.g., www.example.com → http://www.example.com).
- Ensures that URLs with existing protocols (e.g., ftp://example.com, file:///path/to/file) are not modified.
Tests:
- Added new test cases for relative URLs, absolute URLs, and other protocols.
- Fixed normalization issue for file URLs in tests (e.g., file:///path/to/file → file:/path/to/file).

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (if change is visible to the user)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

src/main/java/org/jabref/logic/importer/util/FileFieldParser.java

Signed-off-by: whatisfahad <fahadzamansid@gmail.com>

…ldParser

… URL validation

Signed-off-by: whatisfahad <fahadzamansid@gmail.com>

…ldParser

… URL validation

src/main/java/org/jabref/logic/util/URLUtil.java

…ethod

…bref into fix-for-issue-12186

src/main/java/org/jabref/logic/util/URLUtil.java

src/test/java/org/jabref/logic/net/URLUtilTest.java

src/main/java/org/jabref/logic/util/URLUtil.java

… handling

…bref into fix-for-issue-12186

src/test/java/org/jabref/logic/net/URLUtilTest.java

…a variable

jablib/src/main/java/org/jabref/logic/importer/util/FileFieldParser.java

koppor · 2025-05-04T05:57:50Z

Something seems to be messed up in protocol handling - especially if other protocols than http(s) are used:

> Task :jablib:test
org.jabref.logic.net.URLUtilTest

  createShouldHandleURLs(String, String)

    Test [1] www.example.com, https://www.example.com/ FAILED

    org.opentest4j.AssertionFailedError: expected: <www.example.com> but was: <https://www.example.com/>

    Test [2] example.com, https://example.com/ FAILED

    org.opentest4j.AssertionFailedError: expected: <example.com> but was: <https://example.com/>


  createShouldHandleOtherProtocols(String, String)

    Test [2] file:///path/to/file, file:/path/to/file FAILED

    org.opentest4j.AssertionFailedError: expected: <file:///path/to/file> but was: <https://file:/path/to/file>

…ation

jablib/src/main/java/org/jabref/logic/importer/util/FileFieldParser.java

…arser

trag-bot · 2025-05-04T13:22:01Z

@trag-bot didn't find any issues in the code! ✅✨

jabref-machine · 2025-05-04T13:26:10Z

Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / Checkstyle (pull_request)" and click on it.

In case of issues with the import order, double check that you activated Auto Import. You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues, commit, and push.

koppor · 2025-05-04T13:48:04Z

src/test/java/org/jabref/logic/net/URLUtilTest.java

+    @ParameterizedTest
+    @CsvSource({
+    // Relative URLs (should default to HTTPS)
+    "www.example.com, https://www.example.com",


I don't get this - maybe it should be reversed: expected is first, input is second

ChatGPT and Co-Pilot always generate this ordering; I don't know why ^^

Hi @koppor, I really appreciate the learning experience and your feedback throughout this process. Since this was my first open-source contribution, I gave it my best shot but I realize I couldn’t meet the expectations fully. I’ve decided to step back and will be closing my PR. Thank you again for the opportunity and guidance.

sfahaddev force-pushed the fix-for-issue-12186 branch from 33a8fbf to e060945 Compare March 22, 2025 22:13

Siedlerchr reviewed Mar 22, 2025

View reviewed changes

src/main/java/org/jabref/logic/importer/util/FileFieldParser.java Show resolved Hide resolved

Siedlerchr reviewed Mar 22, 2025

View reviewed changes

src/main/java/org/jabref/logic/importer/util/FileFieldParser.java Outdated Show resolved Hide resolved

sfahaddev added 3 commits March 24, 2025 03:33

Fix URL validation and handling in URLUtil

aed917c

Signed-off-by: whatisfahad <fahadzamansid@gmail.com>

refactor: Replace "://" with PROTOCOL_SEPARATOR constant

4b99f41

refactor: Remove duplicate HTTP and HTTPS prefix constants in FileFie…

4ea9e06

…ldParser

sfahaddev force-pushed the fix-for-issue-12186 branch from 05de4cd to 4ea9e06 Compare March 23, 2025 22:34

sfahaddev added 7 commits March 24, 2025 04:09

refactor: Simplify LinkedFile handling in FileFieldParser and improve…

5f22761

… URL validation

refactor: Clean up whitespace and improve comments in FileFieldParser

04b92fe

Fix URL validation and handling in URLUtil

52bba0f

Signed-off-by: whatisfahad <fahadzamansid@gmail.com>

refactor: Replace "://" with PROTOCOL_SEPARATOR constant

fb8324b

refactor: Remove duplicate HTTP and HTTPS prefix constants in FileFie…

5900647

…ldParser

refactor: Simplify LinkedFile handling in FileFieldParser and improve…

fabfb1d

… URL validation

refactor: Clean up whitespace and improve comments in FileFieldParser

eaf63e3

sfahaddev force-pushed the fix-for-issue-12186 branch from 04b92fe to eaf63e3 Compare March 23, 2025 23:16

refactor: Remove unnecessary whitespace in FileFieldParser

de62e9e

koppor reviewed Mar 23, 2025

View reviewed changes

src/main/java/org/jabref/logic/util/URLUtil.java Show resolved Hide resolved

JabRef deleted a comment from github-actions bot Mar 24, 2025

sfahaddev and others added 6 commits March 25, 2025 04:11

refactor: Update URL validation logic in isURL method to use create m…

106f978

…ethod

Merge branch 'fix-for-issue-12186' of https://github.com/sfahaddev/ja…

598e04b

…bref into fix-for-issue-12186

fixed typo

0ff2957

reuse of isur l

d92779c

reuse of isurl

2bed0cb

Merge branch 'JabRef:main' into fix-for-issue-12186

ab63569

sfahaddev requested review from Siedlerchr and koppor March 24, 2025 23:28

JabRef deleted a comment from koppor Mar 24, 2025

sfahaddev added 3 commits March 25, 2025 04:36

Merge branch 'fix-for-issue-12186' of https://github.com/sfahaddev/ja…

d7967b6

…bref into fix-for-issue-12186

refactor: update isURL method to use createUri for better URL handling

ee6fb59

fix: enhance URL validation by catching MalformedURLException

b67ea74

fix: remove unnecessary whitespace in URLUtil class

a2fa48e

trag-bot bot reviewed Mar 26, 2025

View reviewed changes

src/main/java/org/jabref/logic/util/URLUtil.java Outdated Show resolved Hide resolved

fix: remove FTP handling from URLUtil.create method

95e367f

sfahaddev requested review from InAnYan and koppor March 26, 2025 23:44

koppor requested changes Mar 28, 2025

View reviewed changes

src/main/java/org/jabref/logic/util/URLUtil.java Outdated Show resolved Hide resolved

src/main/java/org/jabref/logic/util/URLUtil.java Outdated Show resolved Hide resolved

src/test/java/org/jabref/logic/net/URLUtilTest.java Outdated Show resolved Hide resolved

sfahaddev added 2 commits March 30, 2025 17:55

fix: enhance URL validation and streamline URL creation tests

8dfbe88

Merge branch 'main' into fix-for-issue-12186

0368f71

trag-bot bot reviewed Mar 30, 2025

View reviewed changes

src/main/java/org/jabref/logic/util/URLUtil.java Show resolved Hide resolved

sfahaddev added 2 commits March 30, 2025 18:19

fix: improve documentation for URL creation method regarding protocol…

8b50c09

… handling

Merge branch 'fix-for-issue-12186' of https://github.com/sfahaddev/ja…

fce4ddc

…bref into fix-for-issue-12186

sfahaddev requested a review from koppor March 30, 2025 13:36

koppor requested changes Apr 17, 2025

View reviewed changes

src/test/java/org/jabref/logic/net/URLUtilTest.java Outdated Show resolved Hide resolved

src/test/java/org/jabref/logic/net/URLUtilTest.java Outdated Show resolved Hide resolved

src/test/java/org/jabref/logic/net/URLUtilTest.java Outdated Show resolved Hide resolved

sfahaddev added 2 commits May 1, 2025 16:28

fix: correct parameter order in URL handling tests for consistency

8374633

Merge branch 'main' into fix-for-issue-12186

3cee13c

sfahaddev force-pushed the fix-for-issue-12186 branch from ac5292b to 3cee13c Compare May 1, 2025 11:36

test: add comprehensive tests for URLUtil methods

65570d6

trag-bot bot reviewed May 1, 2025

View reviewed changes

src/test/java/org/jabref/logic/net/URLUtilTest.java Show resolved Hide resolved

trag-bot bot reviewed May 1, 2025

View reviewed changes

src/test/java/org/jabref/logic/net/URLUtilTest.java Show resolved Hide resolved

sfahaddev added 2 commits May 1, 2025 16:50

fix: remove trailing whitespace in URLUtilTest.java

95f2f5e

fix: improve readability of URL validation test by storing result in …

20ebfaf

…a variable

koppor requested changes May 4, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/importer/util/FileFieldParser.java Outdated Show resolved Hide resolved

fix: replace URLUtil.isURL with LinkedFile.isOnlineLink for URL valid…

dfe5406

…ation

trag-bot bot reviewed May 4, 2025

View reviewed changes

jablib/src/main/java/org/jabref/logic/importer/util/FileFieldParser.java Outdated Show resolved Hide resolved

fix: improve logging message for handling invalid paths in FileFieldP…

7465a15

…arser

koppor reviewed May 4, 2025

View reviewed changes

sfahaddev closed this May 17, 2025

Uh oh!

Conversation

sfahaddev commented Mar 22, 2025

Describe the changes you have made here: what, where, why, ...

Changes:

Mandatory checks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koppor commented May 4, 2025

Uh oh!

Uh oh!

trag-bot bot commented May 4, 2025

Uh oh!

jabref-machine commented May 4, 2025

Uh oh!

koppor May 4, 2025

Choose a reason for hiding this comment

Uh oh!

koppor May 12, 2025

Choose a reason for hiding this comment

Uh oh!

sfahaddev May 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments